Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CL/HIER: Add allgatherv #1050

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

CL/HIER: Add allgatherv #1050

wants to merge 2 commits into from

Conversation

nsarka
Copy link
Collaborator

@nsarka nsarka commented Dec 2, 2024 •
edited
Loading

This PR adds CL/HIER allgatherv and updates the allgatherv gtest to test non-contiguous dst buffers. It is meant to be used in conjunction with my TL/SHM gatherv implementation.

The algorithm is:

  • Node-level gatherv, all sendbufs on the node are put into a scratch buffer
  • Leader-level allgatherv on the scratch buffer, the result is put into the dst buffer
  • Node-level in-place bcast on the dst buffer
  • If the buffer is non-contig, unpack by memcpying the packed buffer into the right displacements

Comparison Data

2 nodes, 32 PPN on lego-grace cg (1 socket, 72 cores/socket)

Size UCC after PR (cl_hier + tl_shm + tl_ucp) UCC before PR (cl_basic + tl_ucp) HCOLL %-Gain, After vs. HCOLL %-Gain, Old vs. HCOLL %-Gain, Before vs. After
1 7.37 19.88 7.41 0.54% -168.29% 62.93%
2 7.81 19.75 7.47 -4.55% -164.39% 60.46%
4 7.62 19.79 7.65 0.39% -158.69% 61.50%
8 8.35 19.78 8.13 -2.71% -143.30% 57.79%
16 9.49 19.8 9.44 -0.53% -109.75% 52.07%
32 9.34 22.91 9.67 3.41% -136.92% 59.23%
64 9.17 22.96 10.03 8.57% -128.91% 60.06%
128 10.47 27.55 10.88 3.77% -153.22% 62.00%
256 12.68 33.37 12.46 -1.77% -167.82% 62.00%
512 19.57 180 25.31 22.68% -611.18% 89.13%
1024 25.38 189.21 308.33 91.77% 38.63% 86.59%
2048 38.86 198.27 311.55 87.53% 36.36% 80.40%

@nsarka nsarka self-assigned this Dec 2, 2024
@nsarka nsarka force-pushed the nsarka/cl-hier-allgatherv branch from 9e34573 to c126e93 Compare December 2, 2024 15:32
@nsarka nsarka force-pushed the nsarka/cl-hier-allgatherv branch 2 times, most recently from 198da51 to 862cad6 Compare December 2, 2024 15:46
@x41lakazam x41lakazam mentioned this pull request Dec 2, 2024
Closed
@nsarka nsarka force-pushed the nsarka/cl-hier-allgatherv branch 2 times, most recently from 0774618 to 1298163 Compare December 6, 2024 22:23
@swx-jenkins3
Copy link

Can one of the admins verify this patch?

@manjugv
Copy link
Contributor

manjugv commented Dec 11, 2024

@aamirshafi Please review

@nsarka nsarka force-pushed the nsarka/cl-hier-allgatherv branch from 584b1b4 to 0305f14 Compare December 13, 2024 21:53
@ikryukov ikryukov self-requested a review December 19, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sergei-Lebedev Sergei-Lebedev Awaiting requested review from Sergei-Lebedev

@janjust janjust Awaiting requested review from janjust

@ikryukov ikryukov Awaiting requested review from ikryukov

At least 2 approving reviews are required to merge this pull request.

Assignees

@nsarka nsarka

Labels
Ready-for-Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nsarka @swx-jenkins3 @manjugv